fix(factory): confirm a freshly-started CLI mount via daemon.pid (follow-up to #12)#13
Conversation
Follow-up to #12. That PR fixed `checkMountStaleness` to honor `daemon.pid`, but `waitForStateFile`/`isValidMountState` (the post-spawn readiness check) still required a top-level `pid`. A CLI-daemonized mount (`relayfile start --background`) records its pid under `daemon.pid`, so after the factory spawns the mount the validator rejected the freshly-written state.json and timed out: `relayfile mount did not become ready within 10000ms (state file is malformed…)`, leaving the factory to warn "writeback may not propagate" even though the mount came up fine. Export `coercePid` and use `coercePid(state.pid) ?? coercePid(state.daemon?.pid)` in `isValidMountState`, matching `checkMountStaleness`. 526 tests green; build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesCLI daemon PID support and stale-mount refresh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the mount preflight check to support CLI-daemonized mounts, which store their process ID (PID) under 'daemon.pid' instead of a top-level 'pid' field in the state file. This is achieved by exporting and utilizing the 'coercePid' helper function in 'isValidMountState' to resolve the PID from either location. A corresponding unit test has been added to verify this behavior. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Follow-up to #12.
Problem
#12 fixed
checkMountStalenessto honordaemon.pid, but the other pid reader —waitForStateFile→isValidMountStateinlocal-mount-preflight.ts, which confirms a mount is ready right after the factory spawns it — still required a top-levelpid. A CLI-daemonized mount (relayfile start --background) records its pid underdaemon.pid, so the validator rejected the freshly-written state.json and timed out:The factory then warned
writeback may not propagateeven though the mount came up fine. Observed live driving the local E2E (relayfile 0.10.5, after relayfile#321 unblocked the cred path).Fix
Export
coercePidfromrelayfile-binary.tsand usecoercePid(state.pid) ?? coercePid(state.daemon?.pid)inisValidMountState, matchingcheckMountStalenessfrom #12. The two pid readers now agree.Tests
local-mount-preflight.test.ts: new case — a CLI mount that writesdaemon: { pid }(no top-levelpid) is confirmed ready (ensureLocalMountresolves). 526 tests green; build clean.🤖 Generated with Claude Code
Summary by cubic
Accept
daemon.pidduring mount state validation so freshly started CLI mounts are confirmed immediately and don’t time out. Aligns the post-spawn readiness check with the staleness check to stop false factory warnings afterrelayfile start --background.coercePidand usedcoercePid(state.pid) ?? coercePid(state.daemon?.pid)inisValidMountState.daemon.pidis accepted.Written for commit 56bffd9. Summary will update on new commits.